[VL] Respect INSTALL_PREFIX in Velox native builds by jackylee-ch · Pull Request #12331 · apache/gluten

jackylee-ch · 2026-06-22T12:28:51Z

What changes are proposed in this pull request?

This PR narrows the native build fix to INSTALL_PREFIX propagation and macOS-only /usr/local protection.

Propagate user-provided INSTALL_PREFIX to native CMake through CMAKE_PREFIX_PATH and CMAKE_INSTALL_PREFIX for dependency builds, Velox, and Gluten CPP. The Arrow dependency build reaches this through the shared cmake_install helper.
On Linux, keep the existing system discovery behavior. No default ignore of /usr or /usr/local is added; an explicit INSTALL_PREFIX is only forwarded to CMake.
On macOS, when the effective INSTALL_PREFIX is outside /usr/local, keep /usr/local out of CMake package discovery and demote /usr/local/include so Apple clang does not prefer stale headers from /usr/local over the chosen prefix. When INSTALL_PREFIX is /usr/local or under it, /usr/local stays visible.
Patch fetched Velox setup scripts before --run_setup_script=ON builds dependencies, so Folly and related setup work use the same prefix policy. The Folly jemalloc flag only prevents setup-time auto-enablement from Homebrew headers; Gluten allocator behavior is not changed.
Resolve thrift from CMAKE_INSTALL_PREFIX/CMAKE_PREFIX_PATH before falling back to /usr/local.

How was this patch tested?

git diff --check HEAD^ HEAD
bash -n dev/build-helper-functions.sh dev/builddeps-veloxbe.sh ep/build-velox/src/build-velox.sh ep/build-velox/src/get-velox.sh
macOS arm64 local build with --run_setup_script=ON and --enable_vcpkg=OFF:
env -u INSTALL_PREFIX PROMPT_ALWAYS_RESPOND=y ./dev/buildbundle-veloxbe.sh --enable_vcpkg=OFF --run_setup_script=ON --enable_s3=OFF --enable_gcs=OFF --enable_hdfs=OFF --enable_abfs=OFF --spark_version=3.5 --velox_home=/private/tmp/gluten-macos-latest.Ze2iov/velox_ep
This completed setup dependencies, Arrow, Velox, Gluten CPP, and Maven packaging with BUILD SUCCESS. Unit tests were skipped by the build command.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: OpenAI Codex (GPT-5)

Copilot

Pull request overview

Introduces a unified native-build “component isolation” resolver (dev/build-isolation.sh) and wires it into the Velox/Arrow build entrypoints so that path-discovery isolation (CMake + compiler include search demotion) is consistently applied across macOS and Linux.

Changes:

Add dev/build-isolation.sh to compute isolation policy, generate a toolchain fragment, and export discovery/ignore flags for all nested CMake builds.
Update native build entrypoints (builddeps-veloxbe.sh, build-velox.sh, build-helper-functions.sh) to consume the resolver outputs instead of duplicating macOS-only /usr/local ignore logic.
Add a dry-run scenario test harness (dev/tests/test-build-isolation.sh) and ignore generated artifacts via .gitignore; harden dev/build-arrow.sh’s managed-dir cleanup behavior.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ep/build-velox/src/build-velox.sh	Sources the isolation resolver and injects resolved CMake/compiler isolation flags into Velox’s build.
dev/tests/test-build-isolation.sh	Adds dry-run tests validating resolved isolation behavior and propagation.
dev/builddeps-veloxbe.sh	Establishes/install-prefix explicitness and applies resolver-derived CMake isolation across native build stages.
dev/build-isolation.sh	New single-source resolver that generates toolchain/env artifacts and exports isolation flags/toolchain.
dev/build-helper-functions.sh	Uses the resolver to apply isolation flags consistently for CMake-based dependency builds.
dev/build-arrow.sh	Avoids wiping user-provided Arrow prefixes and consults resolver for standalone prefix defaults.
.gitignore	Ignores generated isolation artifacts under `.gluten-build-cache/`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

+  if [ -n "${INSTALL_PREFIX:-}" ]; then
    GLUTEN_CMAKE_OPTIONS+=" -DCMAKE_PREFIX_PATH=$INSTALL_PREFIX"
+    GLUTEN_CMAKE_OPTIONS+=" -DCMAKE_INSTALL_PREFIX=$INSTALL_PREFIX"
+  fi


Propagate a user-provided INSTALL_PREFIX into the native Velox build flow by exporting it on Linux when set and passing it to CMake as CMAKE_PREFIX_PATH/CMAKE_INSTALL_PREFIX for dependency builds, Velox, and Gluten CPP. This keeps Linux's existing system-path discovery unchanged unless the user explicitly chooses a prefix. On macOS, keep the existing /usr/local isolation only when the effective INSTALL_PREFIX is outside /usr/local. Also demote /usr/local/include in compiler flags so Apple clang does not prefer stale system headers over the selected install prefix, and patch fetched Velox setup scripts before --run_setup_script=ON builds dependencies such as Folly. Teach the Gluten Velox module to resolve thrift from CMAKE_INSTALL_PREFIX/CMAKE_PREFIX_PATH before falling back to /usr/local.

jackylee-ch · 2026-07-01T10:21:57Z

cc @zhouyuan @philo-he

zhouyuan

👍

philo-he

LGTM. Just one follow-up suggestion. Thanks.

prestodb-ci · 2026-07-02T02:40:40Z

Rebase job

…apache#12331) PR apache#12331 (ce411ad) made INSTALL_PREFIX flow into the Arrow, Velox and Gluten CPP CMake entry points. This follow-up keeps that behavior unchanged and fixes what is still broken when building with --build_type=Debug, --build_tests=ON or --build_benchmarks=ON on macOS: - Replace the "-isystem /usr/local/include" demotion with an SDK sysroot (SDKROOT). Demotion keeps /usr/local/include in the search path ahead of imported targets' system includes, so test builds can compile against /usr/local headers (folly, gtest) while linking INSTALL_PREFIX libraries. Selecting the SDK sysroot removes /usr/local/include from the default search path entirely; package discovery isolation via CMAKE_IGNORE_* is unchanged. - Restore BUILD_TYPE after sourcing build-arrow.sh, which overwrote Debug with Release for the rest of the build. - Force-load the Velox mono archive into libvelox.dylib and link facebook::velox privately on macOS so test executables resolve Velox symbols from the dylib instead of linking a second copy of the archive. - Rewrite folly's imported interface to use gflags_shared on macOS. Folly exports gflags_static while glog links the shared gflags, and loading both copies makes gflags register built-in flags twice and abort test binaries. Done in Gluten's CMake at find_package(Folly) time instead of patching installed folly-targets.cmake. - Resolve test data files from a compile-time source-dir macro instead of the working directory, and bump gtest discovery timeout; run the ld/ldd post-build checks only on Linux. Linux discovery logic and the Folly no-jemalloc setup from apache#12331 are unchanged.

…#12331) (#12470)

Copilot AI review requested due to automatic review settings June 22, 2026 12:28

github-actions Bot added BUILD VELOX labels Jun 22, 2026

Copilot started reviewing on behalf of jackylee-ch June 22, 2026 12:34 View session

Copilot AI reviewed Jun 22, 2026

View reviewed changes

Comment thread dev/build-arrow.sh Outdated

Comment thread dev/build-isolation.sh Outdated

Comment thread dev/build-isolation.sh Outdated

jackylee-ch marked this pull request as draft June 22, 2026 13:47

jackylee-ch force-pushed the gluten-native-build-isolation branch 3 times, most recently from 51280f4 to 44b67a8 Compare June 23, 2026 07:29

jackylee-ch force-pushed the gluten-native-build-isolation branch from 44b67a8 to c6f0328 Compare July 1, 2026 02:23

jackylee-ch changed the title ~~[VL] Unify native-build component isolation via a single resolver (macOS + Linux)~~ [VL] Respect INSTALL_PREFIX in Velox native builds Jul 1, 2026

jackylee-ch mentioned this pull request Jul 1, 2026

[VL] Fix gflags dual-registration abort on macOS arm64 #12100

Closed

jackylee-ch marked this pull request as ready for review July 1, 2026 07:10

Copilot AI review requested due to automatic review settings July 1, 2026 07:10

Copilot started reviewing on behalf of jackylee-ch July 1, 2026 07:10 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

jackylee-ch force-pushed the gluten-native-build-isolation branch from c6f0328 to 6304aa8 Compare July 1, 2026 07:25

zhouyuan approved these changes Jul 1, 2026

View reviewed changes

philo-he approved these changes Jul 1, 2026

View reviewed changes

Comment thread ep/build-velox/src/get-velox.sh

jackylee-ch merged commit ce411ad into apache:main Jul 2, 2026
60 checks passed

zhouyuan mentioned this pull request Jul 2, 2026

[VL] Include headers from INSTALL_PREFIX as system includes on Mac #12423

Closed

jackylee-ch mentioned this pull request Jul 7, 2026

[VL] Support test/debug/benchmark native builds on macOS (follow-up to #12331) #12470

Merged

jackylee-ch added a commit that referenced this pull request Jul 9, 2026

[VL] Support test/debug/benchmark native builds on macOS (follow-up to …

8aeff65

…#12331) (#12470)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[VL] Respect INSTALL_PREFIX in Velox native builds#12331

[VL] Respect INSTALL_PREFIX in Velox native builds#12331
jackylee-ch merged 1 commit into
apache:mainfrom
jackylee-ch:gluten-native-build-isolation

jackylee-ch commented Jun 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

jackylee-ch commented Jul 1, 2026

Uh oh!

zhouyuan left a comment

Uh oh!

philo-he left a comment

Uh oh!

Uh oh!

Uh oh!

prestodb-ci commented Jul 2, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

jackylee-ch commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed in this pull request?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

jackylee-ch commented Jul 1, 2026

Uh oh!

zhouyuan left a comment

Choose a reason for hiding this comment

Uh oh!

philo-he left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

prestodb-ci commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jackylee-ch commented Jun 22, 2026 •

edited

Loading

prestodb-ci commented Jul 2, 2026 •

edited

Loading